Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: max-specificity rule #24

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: max-specificity rule #24

wants to merge 2 commits into from

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Nov 27, 2024

Prerequisites checklist

What is the purpose of this pull request?

Implement a max-specificity rule.

What changes did you make? (Give an overview)

  • Created max-specificity rule
  • Added tests
  • Added to plugin exports

Related Issues

fixes #19

Is there anything you'd like reviewers to focus on?

This is a rough draft, I'd like some feedback on the design.

  • Does it make sense to have people write out a, b, and c to match the spec? Or would it better to just have them enter an array?
  • Would any other options make sense, in which case, the desired specificity should be one key in an options object?

@nzakas nzakas mentioned this pull request Nov 27, 2024
1 task
@jeddy3
Copy link

jeddy3 commented Nov 28, 2024

We added selector-max-specificity to Stylelint before the introduction of specificity-changing pseudo-classes like :is(), :where(), :not() and :has(). Having thought about it some more, I'm not sure the rule is useful these days.

Due to these pseudo-classes, it makes a poor proxy for controlling selector complexity and for disallowing ID selectors. For example, both these selectors have a specificity of 0,0,0:

:where(#id),
:where(a.complex[attr], #id) {}

And makes a poorer proxy for disallowing other types of selectors as it groups them:

  • B = the number of class selectors, attributes selectors, and pseudo-classes
  • C = the number of type selectors and pseudo-elements

It'd be good to hear if people have another use for this rule.

If you do keep it, the implementation should probably consider these specificity-changing pseudo-classes and nesting, which is Baseline 2023.

In Stylelint, we use @csstools/selector-specificity for calculating the specificity, which supports the specificity-changing pseudo-classes and elements. We also desugar nested selectors before passing them to the calculator. However, we don't yet desugar to :is() (as our current implementation predates the latest CSS nesting spec):

a, b { 
  & c {} 
}

/* should be desugared to */
:is(a, b) c {}

/* be we currently desugar to */
a c, b c {}

This can lead to different specificities, and our calculations may not match the browser.

@nzakas
Copy link
Member Author

nzakas commented Dec 2, 2024

Interesting. I think I'll need to dig into this a bit more.

@lahmatiy
Copy link

lahmatiy commented Dec 6, 2024

Take a look at bramus/specificity. It appears to to cover all the key aspects of selector specificity calculation according to the latest CSS specifications. Since it’s built on top of CSSTree, it might be a good candidate for adoption instead of implementing a custom specificity calculation from scratch (see calculate.js).

@nzakas
Copy link
Member Author

nzakas commented Dec 6, 2024

@lahmatiy thanks! We're actually discussing over on #19 whether or not specificity even makes sense as a measure of selector complexity anymore. Would love to hear your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Blocked
Development

Successfully merging this pull request may close these issues.

New Rule: no-id-selector
3 participants